Skip to content

Conversation

@oliwoli
Copy link
Contributor

@oliwoli oliwoli commented Mar 18, 2025

3 Files are broken in 1.20.5:

  • RuleBookGUI.java
  • TestUtils.java
  • TestUtilsClient.java

For example ClientPlayNetworking.send(TestUtils.SET_TIME_C2S, passedData); doesn't work anymore because

The method send(CustomPayload) in the type ClientPlayNetworking is not applicable for the arguments (Identifier, PacketByteBuf)Java(67108979)
void net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking.send(CustomPayload payload)

If you have time to fix it @Fourmisain 😅 But don't feel obliged

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @oliwoli, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request updates the project to Minecraft version 1.20.5. It seems like the author, oliwoli, is working on fixing broken files in the 1.20.5 version, specifically mentioning RuleBookGUI.java, TestUtils.java, and TestUtilsClient.java. The description highlights a breaking change in how ClientPlayNetworking.send is used. The changes involve updating the Minecraft version and related dependencies in minepkg.toml and gradle.properties.

Highlights

  • Version Update: The Minecraft version has been updated from 1.20.2/1.20.4 to 1.20.5 in gradle.properties and minepkg.toml.
  • Dependency Updates: Fabric version and related mappings have been updated to be compatible with Minecraft 1.20.5.
  • Potential Breaking Changes: The pull request description mentions that ClientPlayNetworking.send is no longer working as expected in 1.20.5, indicating a need for code adjustments in the affected files.

Changelog

Click here to see the changelog
  • .minepkg-lock.toml
    • Updated Minecraft version from 1.20.4 to 1.20.2.
    • Updated fabric version from 0.97.2+1.20.4.mpkg.1 to 0.91.6+1.20.2.mpkg.1.
    • Updated mapping from 1.20.4+build.3 to 1.20.2+build.4.
  • gradle.properties
    • Updated Minecraft version from 1.20.2 to 1.20.5.
    • Updated yarn mappings from 1.20.2+build.4 to 1.20.5+build.1.
    • Updated fabric version from 0.91.6+1.20.2 to 0.97.8+1.20.5.
    • Updated minecraft_version_range from >=1.20.2 <=1.20.4 to ~1.20.5.
  • minepkg.toml
    • Updated Minecraft version requirement from >=1.20.2 <=1.20.4 to ~1.20.5.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Did you know?

The Fabric mod loader for Minecraft is designed to be lightweight and modular, allowing for faster updates and greater flexibility compared to other mod loaders.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request updates the mod to Minecraft version 1.20.5. This involves changes to the .minepkg-lock.toml, gradle.properties, and minepkg.toml files to reflect the new version and dependencies. The core logic of the mod remains unchanged, focusing on version compatibility.

Merge Readiness

The pull request updates the mod to Minecraft version 1.20.5. Given the limited scope of changes and the absence of any identified high or critical severity issues, the pull request appears to be in good shape to be merged. However, it's advisable to have another reviewer examine the changes to ensure compatibility and stability with the updated Minecraft version. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging.

@Fourmisain
Copy link
Contributor

Will take a bit longer as the networking needs to be rewritten.

@oliwoli
Copy link
Contributor Author

oliwoli commented Mar 18, 2025

Will take a bit longer as the networking needs to be rewritten.

😎🙏 alright! Ofc, no worries.

@Fourmisain
Copy link
Contributor

Should be good to go!

Turns out there's no LibGui for 1.20.5 - which is actually a good thing as that's a garbage version anyway.
(The only difference to 1.20.6 is that a world migration bug has been fixed, literally 0 reason to ever go 1.20.5.)

Didn't touch the .minepkg-lock.toml, could only edit it manually anyway. (Also, is that file even supposed to be in the repo?)

@Fourmisain Fourmisain changed the title 1.20.5 wip (don't merge) update to 1.20.6 Mar 18, 2025
@oliwoli
Copy link
Contributor Author

oliwoli commented Mar 18, 2025

Okay damn, this is what you call "will take a bit longer"? 😆 That was hella quick

@Fourmisain
Copy link
Contributor

What do you mean? It took so long, my apple pie is already gone!

@oliwoli
Copy link
Contributor Author

oliwoli commented Mar 18, 2025

Didn't touch the .minepkg-lock.toml, could only edit it manually anyway. (Also, is that file even supposed to be in the repo?)

Yeah that's fine. If it were up to me, I'd gitignore it but I think I remember @fiws saying it's the same principle as a package-lock.json file when you use npm and you usually include that in commits too (can't really explain why though).

Edit: some dude on stackoverflow said:

Bear in mind that your package.json contains only your direct dependencies, not the dependencies of your dependencies (sometimes called nested or transitive dependencies). This means with the standard package.json you can't control the versions of those nested dependencies, referencing them directly or as peer dependencies won't help as you also don't control the version tolerance that your direct dependencies define for these nested dependencies.

Even if you lock down the versions of your direct dependencies you cannot 100% guarantee that your full dependency tree will be identical every time. Secondly you might want to allow non-breaking changes (based on semantic versioning) of your direct dependencies which gives you even less control of nested dependencies plus you again can't guarantee that your direct dependencies won't at some point break semantic versioning rules themselves.

The solution to all this is the package-lock.json file which as described above locks in the versions of the full dependency tree. This allows you to guarantee your dependency tree for other developers or for releases, whilst still allowing testing of new dependency versions (direct or indirect) using your standard package.json.

So... I guess when things get complicated a lock file makes sense.

@oliwoli
Copy link
Contributor Author

oliwoli commented Mar 18, 2025

I think you could run minepkg install to update the lock file or does that also require you to authenticate yourself?

@Fourmisain
Copy link
Contributor

That seemed to do it.
I think I'll squash some of the commits here - or maybe just the whole PR?
I remember squashing a PR essentially removes all attributions though (since there'll only be one commit, it's either you or me, not both), which always annoyed me.

@oliwoli
Copy link
Contributor Author

oliwoli commented Mar 18, 2025

That's fine with me, you're the one doing all the hard work anyway :P

@Fourmisain Fourmisain marked this pull request as ready for review March 18, 2025 20:01
@Fourmisain
Copy link
Contributor

I'm not even sure how the commit message would look like though, I'll look up if it can be set somehow.

@Fourmisain
Copy link
Contributor

default

default is default
default is default!

@Fourmisain
Copy link
Contributor

Oh well, let's just see what happens.
If everything breaks and the apocalypse starts we can still rewrite the history.

@Fourmisain
Copy link
Contributor

Aha, there's a popup that says "This commit will be authored by Oli" and it let's you choose the commit message and extended description, where it also says "Co-authored-by: Fourmisain", looks fine, so here we go.

@Fourmisain Fourmisain merged commit d09d92d into main Mar 18, 2025
1 check passed
@Fourmisain Fourmisain deleted the 1.20.5-wip branch March 18, 2025 20:14
@oliwoli
Copy link
Contributor Author

oliwoli commented Mar 18, 2025

Oh, ok!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants